Skip to content

replace babel require hook with a build step #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 5, 2015

Conversation

joeybaker
Copy link
Contributor

The babel require hook is explicitly not suitable for libraries:
https://babeljs.io/docs/usage/require/

I’ve noticed that in trying to use this module, and trying to use the
babel require hook as well, babel stops working. This patch removes the
require hook, adds a build process, and runs that build process before
a npm publish and npm test

@mightyaleksey
Copy link
Member

Thank you for your help :)
I put it as a temporary solution for projects, which doesn't use babel, because I was uncertain how to compile it.

@joeybaker
Copy link
Contributor Author

Of course! Thanks for building this!

@joeybaker
Copy link
Contributor Author

FYI: there are failing tests in master right now. I was a bit unsure if that was intentional?

ignore: false,
loose: 'all'
});

module.exports = require('./src');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to fix path to the dir here? Looks like we will get dist after compile?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, you're right. That slipped through, my bad!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

The babel require hook is explicitly not suitable for libraries:
https://babeljs.io/docs/usage/require/ 

I’ve noticed that in trying to use this module, and trying to use the 
babel require hook as well, babel stops working. This patch removes the
require hook, adds a build process, and runs that build process before
a `npm publish` and `npm test`
@mightyaleksey
Copy link
Member

Since I found out that I have same goals here as in css-modules-loader-core, I decided to copy all test-cases from that lib and run css-modules-require-hook through them. I managed to fix few, but some of them still broken. So its ok :)

@mightyaleksey
Copy link
Member

Looks like its ok now, thank you for your help again :)

mightyaleksey added a commit that referenced this pull request Aug 5, 2015
replace babel require hook with a build step
@mightyaleksey mightyaleksey merged commit 9b78d17 into css-modules:master Aug 5, 2015
@joeybaker joeybaker deleted the removeRequireHook branch August 5, 2015 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants